Skip to content

For msm/python#13

Open
quic-bjorande wants to merge 5 commits into
linux-msm:masterfrom
quic-bjorande:for-msm/python
Open

For msm/python#13
quic-bjorande wants to merge 5 commits into
linux-msm:masterfrom
quic-bjorande:for-msm/python

Conversation

@quic-bjorande

Copy link
Copy Markdown

No description provided.

The SSH helper assumes ssh-agent is always available and panics when the
agent cannot be contacted. That makes library users fail before they can
try another usable key source.

Try ssh-agent first, but treat agent connection and identity lookup
failures as normal authentication misses. Fall back to SK8BRD_SSH_KEY,
id_ed25519, and id_rsa, and only report failure after every candidate
has been rejected.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The interactive and non-interactive clients drain stderr before they
read the framed protocol from stdout. If the remote server has no
pending stderr data, the clients can block forever and never process
the stdout message that would let the boot flow progress.

Read both SSH streams with tokio::select!, buffer partial stdout
frames, and parse every complete protocol message as it arrives. Track
stderr EOF so the select loop does not spin after the status stream
closes, wake the interactive loop so Ctrl-A q can exit when the
console is idle, and bound the shutdown power-off request so exit does
not hang if the remote writer stalls.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Image upload sends every fastboot payload as a separate 2 KiB protocol
message. With the russh channel writer this creates a large number of
small async writes and mutex acquisitions for large Android boot images.

Use 8 KiB image frames, which remain safely below the cdba-server 16 KiB
receive ring, and hold the writer lock for the duration of an image
transfer.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
sk8brd allowed us to integrate cdba into Rust workflows, without having
to shell out and deal with CLI parsing and process management. In the
same way there are use cases for being able to integrate cdba workflows
directly into Python tools.

Add a PyO3 extension crate that builds the sk8brd_cdba module with
maturin. Expose blocking Python methods for listing devices, booting an
image, and toggling board power while Rust keeps the existing async
protocol handling.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Add a small Python example application that exercises the module
through environment variables and documents the packaging workflow.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
@konradybcio

Copy link
Copy Markdown
Member

This looks really good!

I'll revisit this next week

Comment thread proto/src/ssh.rs
fn expand_tilde(path: String) -> PathBuf {
let path = path.trim().to_string();
if let Some(tail) = path.strip_prefix("~/") {
return std::env::var("HOME")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::env::home_dir() sounds like a good candidate here

Comment thread proto/src/ssh.rs
}
if let Ok(home) = std::env::var("HOME") {
keys.push(Path::new(&home).join(".ssh").join("id_ed25519"));
keys.push(Path::new(&home).join(".ssh").join("id_rsa"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm majorly annoyed the library doesn't do that for us..

Comment thread proto/src/lib.rs
write_msg(&mut *write_sink, Sk8brdMsgs::MsgFastbootDownload, &[]).await
}

pub async fn send_image_quiet(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, let's depend on the pbr crate (the same progressbar we have in qdlrs) and make send_image take some sort of an Option<Sk8brdProgressHandler>

Comment thread python/src/lib.rs
Close,
}

impl CdbaClient {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of these functions seem like they could live in cli/ (is the cli actually useful for any purpose, by the way?) or at least somewhere non-py-specific

```sh
python -m pip install maturin
maturin develop
python examples/python_cdba/boot_linux.py \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

virtualenv + pip install . seems to do the job too

import uuid


def marker_exit_code(output, marker):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some comments and preferably a mypy CI job

return output, result
time.sleep(0.1)

raise TimeoutError(f"timed out waiting for {description}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description?

- `--timeout`, defaults to `120`
- `--prompt`, defaults to `root@qcom-armv8a:~#`
- `--command`, defaults to `uname -a; id`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today sk8brd does while (board_online) { if (fastboot) send_image() }, perhaps making it only try to send the image once could be beneficial for the testing usecases

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Particularly since MsgPowerOn refreshes the timer

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(worth noting the .py file's timer is separate)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this in place to facilitate "reboot" testing, but at the same time I now need to detect when the board reset unexpectedly by looking for early boot logs. Perhaps this can go either way... This was the original behavior of the cdba client as well, but I later made it optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants